feat(apps): introduce read accessor for media calls#40863
Conversation
|
Looks like this PR is not ready to merge, because of the following issues:
Please fix the issues and try again If you have any trouble, please check the PR guidelines |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (2)
📜 Recent review details⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
🧰 Additional context used📓 Path-based instructions (1)**/*.{ts,tsx,js}📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
Files:
🪛 Biome (2.4.16)apps/meteor/app/apps/server/bridges/bridges.js[error] 17-17: Illegal use of an import declaration outside of a module (parse) 🔇 Additional comments (3)
WalkthroughThis PR introduces a new ChangesMediaCallRead Accessor Implementation
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Biome (2.4.16)apps/meteor/app/apps/server/bridges/bridges.jsFile contains syntax errors that prevent linting: Line 1: Illegal use of an import declaration outside of a module; Line 3: Illegal use of an import declaration outside of a module; Line 4: Illegal use of an import declaration outside of a module; Line 5: Illegal use of an import declaration outside of a module; Line 6: Illegal use of an import declaration outside of a module; Line 7: Illegal use of an import declaration outside of a module; Line 8: Illegal use of an import declaration outside of a module; Line 9: Illegal use of an import declaration outside of a module; Line 10: Illegal use of an import declaration outside of a module; Line 11: Illegal use of an import declaration outside of a module; Line 12: Illegal use of an import declaration outside of a module; Line 13: Illegal use of an import declaration outside of a module; Line 14: Illegal use of an import declaration outside of a module; Line 15: Illegal use of an import declaration outside of a module; Line 16: Illegal use ... [truncated 317 characters] ... ne 21: Illegal use of an import declaration outside of a module; Line 22: Illegal use of an import declaration outside of a module; Line 23: Illegal use of an import declaration outside of a module; Line 24: Illegal use of an import declaration outside of a module; Line 25: Illegal use of an import declaration outside of a module; Line 26: Illegal use of an import declaration outside of a module; Line 27: Illegal use of an import declaration outside of a module; Line 28: Illegal use of an import declaration outside of a module; Line 29: Illegal use of an import declaration outside of a module; Line 30: Illegal use of an import declaration outside of a module; Line 31: Illegal use of an import declaration outside of a module; Line 33: Illegal use of an export declaration outside of a module Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
d-gubert
force-pushed
the
feat/apps-media-calls
branch
from
2bc6cd3 to
4126852
Compare
🦋 Changeset detectedLatest commit: 93430f7 The changes in this PR will be included in the next version bump. This PR includes changesets to release 5 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
d-gubert
force-pushed
the
feat/sip-metadata
branch
from
d2a38c3 to
bbd7101
Compare
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## feat/sip-metadata #40863 +/- ##
=====================================================
+ Coverage 69.96% 69.99% +0.03%
=====================================================
Files 3353 3353
Lines 128820 128820
Branches 22342 22306 -36
=====================================================
+ Hits 90132 90172 +40
+ Misses 35399 35359 -40
Partials 3289 3289
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
d-gubert
force-pushed
the
feat/apps-media-calls
branch
from
f85555a to
0fc81ce
Compare
coderabbitai
Bot
added
the
type: feature
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
packages/apps/tests/test-data/bridges/mediaCallBridge.ts (1)
6-8: ⚡ Quick winConsider returning a mock instead of throwing.
The stub currently throws, which will break any test that uses this bridge through
TestsAppBridges. For better test resilience and to match patterns used in other test bridges in this directory, consider returning a mock call fromTestData.getMediaCall().♻️ Proposed implementation returning a mock
+import { TestData } from '../utilities'; + import type { IMediaCall } from '`@rocket.chat/apps-engine/definition/mediaCalls`'; import { MediaCallBridge } from '../../../src/server/bridges'; export class TestsMediaCallBridge extends MediaCallBridge { public getById(callId: string, appId: string): Promise<IMediaCall> { - throw new Error('Method not implemented.'); + return Promise.resolve(TestData.getMediaCall()); } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/apps/tests/test-data/bridges/mediaCallBridge.ts` around lines 6 - 8, The getById method currently throws and breaks tests; change it to return a mock media call by calling TestData.getMediaCall(callId, appId) (or TestData.getMediaCall() if signature differs) and return it as a resolved Promise instead of throwing; update the public getById(callId: string, appId: string): Promise<IMediaCall> implementation to return that mock so TestsAppBridges consumers get a valid IMediaCall object.apps/meteor/tests/end-to-end/apps/app-media-call-reader.ts (1)
12-14: 💤 Low valueVerify test fixture data availability.
The test assumes these call IDs are seeded by
callHistoryTestData.ts. If the seed data is missing or the IDs change, the tests will fail without clear diagnostics. Consider adding an initial query to verify the fixtures exist, or document the dependency clearly.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/meteor/tests/end-to-end/apps/app-media-call-reader.ts` around lines 12 - 14, Tests rely on the fixture IDs INTERNAL_CALL_ID and EXTERNAL_CALL_ID which are expected to be seeded by callHistoryTestData.ts; add an initial verification step at the start of the test (e.g., a query or assertion using the same lookup used in the test harness) that checks those call records exist and fail fast with a clear error message if they are missing, or alternatively document the dependency on callHistoryTestData.ts in the test setup comments so maintainers know to ensure the seed data is present and consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.changeset/tired-teeth-cheat.md:
- Line 7: Update the changelog entry text to use the plural "media calls" for
accuracy: locate the sentence mentioning "Added the new MediaCallRead accessor
that allows apps to read information from the system's media call" and change it
to "Added the new MediaCallRead accessor that allows apps to read information
from the system's media calls" so it reflects that the accessor reads from the
collection of media call records.
In `@packages/apps-engine/src/definition/accessors/IMediaCallRead.ts`:
- Line 14: The return nullability is inconsistent: update the
IMediaCallRead.getById signature to return Promise<IMediaCall | null> (instead
of Promise<IMediaCall | undefined>) so it matches the MediaCallBridge.ts
implementation and the value forwarded by MediaCallRead.ts; then update any
implementations/usages of IMediaCallRead.getById to handle null (or adjust
MediaCallRead.ts to explicitly return null) to ensure consistent null semantics
across IMediaCallRead.getById, MediaCallBridge, and MediaCallRead.
In `@packages/apps/src/server/bridges/MediaCallBridge.ts`:
- Around line 9-17: The bridge currently returns null from doGetById which
conflicts with the accessor contract that expects undefined; update
MediaCallBridge by changing doGetById's and its abstract getById signature to
return Promise<IMediaCall | undefined> instead of Promise<IMediaCall | null>,
and replace the literal return null with return undefined so the sentinel value
matches packages/apps/src/server/accessors/MediaCallRead.ts and callers using
=== undefined remain correct.
---
Nitpick comments:
In `@apps/meteor/tests/end-to-end/apps/app-media-call-reader.ts`:
- Around line 12-14: Tests rely on the fixture IDs INTERNAL_CALL_ID and
EXTERNAL_CALL_ID which are expected to be seeded by callHistoryTestData.ts; add
an initial verification step at the start of the test (e.g., a query or
assertion using the same lookup used in the test harness) that checks those call
records exist and fail fast with a clear error message if they are missing, or
alternatively document the dependency on callHistoryTestData.ts in the test
setup comments so maintainers know to ensure the seed data is present and
consistent.
In `@packages/apps/tests/test-data/bridges/mediaCallBridge.ts`:
- Around line 6-8: The getById method currently throws and breaks tests; change
it to return a mock media call by calling TestData.getMediaCall(callId, appId)
(or TestData.getMediaCall() if signature differs) and return it as a resolved
Promise instead of throwing; update the public getById(callId: string, appId:
string): Promise<IMediaCall> implementation to return that mock so
TestsAppBridges consumers get a valid IMediaCall object.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 1f838a7f-ca6a-412b-886f-0a2109886b30
⛔ Files ignored due to path filters (1)
apps/meteor/tests/data/apps/app-packages/media-call-reader-test_0.0.1.zipis excluded by!**/*.zip
📒 Files selected for processing (27)
.changeset/tired-teeth-cheat.mdapps/meteor/app/apps/server/bridges/bridges.jsapps/meteor/app/apps/server/bridges/mediaCalls.tsapps/meteor/tests/data/apps/app-packages/README.mdapps/meteor/tests/data/apps/app-packages/index.tsapps/meteor/tests/data/apps/helper.tsapps/meteor/tests/end-to-end/apps/app-media-call-reader.tspackages/apps-engine/src/definition/accessors/IMediaCallRead.tspackages/apps-engine/src/definition/accessors/IRead.tspackages/apps-engine/src/definition/accessors/index.tspackages/apps-engine/src/definition/mediaCalls/IMediaCall.tspackages/apps-engine/src/definition/mediaCalls/index.tspackages/apps/deno-runtime/lib/accessors/mod.tspackages/apps/src/AppsEngine.tspackages/apps/src/server/accessors/MediaCallRead.tspackages/apps/src/server/accessors/Reader.tspackages/apps/src/server/accessors/index.tspackages/apps/src/server/bridges/AppBridges.tspackages/apps/src/server/bridges/MediaCallBridge.tspackages/apps/src/server/bridges/index.tspackages/apps/src/server/managers/AppAccessorManager.tspackages/apps/src/server/permissions/AppPermissions.tspackages/apps/tests/server/accessors/MediaCallRead.test.tspackages/apps/tests/server/accessors/Reader.test.tspackages/apps/tests/test-data/bridges/appBridges.tspackages/apps/tests/test-data/bridges/mediaCallBridge.tspackages/apps/tests/test-data/utilities.ts
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: 🔨 Test UI (CE) / MongoDB 8.0 (4/4)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
**/*.{ts,tsx,js}: Write concise, technical TypeScript/JavaScript with accurate typing in Playwright tests
Avoid code comments in the implementation
Files:
packages/apps/src/server/accessors/index.tspackages/apps-engine/src/definition/accessors/index.tspackages/apps/tests/server/accessors/MediaCallRead.test.tspackages/apps-engine/src/definition/accessors/IMediaCallRead.tsapps/meteor/app/apps/server/bridges/mediaCalls.tsapps/meteor/tests/data/apps/app-packages/index.tspackages/apps/src/server/accessors/MediaCallRead.tspackages/apps/src/server/bridges/index.tspackages/apps/src/server/permissions/AppPermissions.tspackages/apps-engine/src/definition/accessors/IRead.tspackages/apps-engine/src/definition/mediaCalls/index.tspackages/apps/tests/server/accessors/Reader.test.tspackages/apps/tests/test-data/utilities.tspackages/apps/src/server/bridges/AppBridges.tspackages/apps/tests/test-data/bridges/mediaCallBridge.tspackages/apps/tests/test-data/bridges/appBridges.tspackages/apps-engine/src/definition/mediaCalls/IMediaCall.tspackages/apps/src/server/bridges/MediaCallBridge.tsapps/meteor/app/apps/server/bridges/bridges.jspackages/apps/src/server/managers/AppAccessorManager.tsapps/meteor/tests/data/apps/helper.tspackages/apps/src/server/accessors/Reader.tspackages/apps/deno-runtime/lib/accessors/mod.tspackages/apps/src/AppsEngine.tsapps/meteor/tests/end-to-end/apps/app-media-call-reader.ts
🧠 Learnings (8)
📚 Learning: 2026-02-26T19:25:44.063Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 38778
File: packages/ui-voip/src/providers/useMediaSession.ts:192-192
Timestamp: 2026-02-26T19:25:44.063Z
Learning: In the Rocket.Chat repository, do not reference Biome lint rules in code review feedback. Biome is not used even if biome.json exists; only reference Biome rules if there is explicit, project-wide usage documented. For TypeScript files, review lint implications without Biome guidance unless the project enables Biome rules.
Applied to files:
packages/apps/src/server/accessors/index.tspackages/apps-engine/src/definition/accessors/index.tspackages/apps/tests/server/accessors/MediaCallRead.test.tspackages/apps-engine/src/definition/accessors/IMediaCallRead.tsapps/meteor/app/apps/server/bridges/mediaCalls.tsapps/meteor/tests/data/apps/app-packages/index.tspackages/apps/src/server/accessors/MediaCallRead.tspackages/apps/src/server/bridges/index.tspackages/apps/src/server/permissions/AppPermissions.tspackages/apps-engine/src/definition/accessors/IRead.tspackages/apps-engine/src/definition/mediaCalls/index.tspackages/apps/tests/server/accessors/Reader.test.tspackages/apps/tests/test-data/utilities.tspackages/apps/src/server/bridges/AppBridges.tspackages/apps/tests/test-data/bridges/mediaCallBridge.tspackages/apps/tests/test-data/bridges/appBridges.tspackages/apps-engine/src/definition/mediaCalls/IMediaCall.tspackages/apps/src/server/bridges/MediaCallBridge.tspackages/apps/src/server/managers/AppAccessorManager.tsapps/meteor/tests/data/apps/helper.tspackages/apps/src/server/accessors/Reader.tspackages/apps/deno-runtime/lib/accessors/mod.tspackages/apps/src/AppsEngine.tsapps/meteor/tests/end-to-end/apps/app-media-call-reader.ts
📚 Learning: 2026-02-26T19:25:44.063Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 38778
File: packages/ui-voip/src/providers/useMediaSession.ts:192-192
Timestamp: 2026-02-26T19:25:44.063Z
Learning: In this repository (RocketChat/Rocket.Chat), Biome lint rules are not used even if a biome.json exists. When reviewing TypeScript files (e.g., packages/ui-voip/src/providers/useMediaSession.ts), ensure lint suggestions do not reference Biome-specific rules. Rely on general ESLint/TypeScript lint rules and project conventions instead.
Applied to files:
packages/apps/src/server/accessors/index.tspackages/apps-engine/src/definition/accessors/index.tspackages/apps/tests/server/accessors/MediaCallRead.test.tspackages/apps-engine/src/definition/accessors/IMediaCallRead.tsapps/meteor/app/apps/server/bridges/mediaCalls.tsapps/meteor/tests/data/apps/app-packages/index.tspackages/apps/src/server/accessors/MediaCallRead.tspackages/apps/src/server/bridges/index.tspackages/apps/src/server/permissions/AppPermissions.tspackages/apps-engine/src/definition/accessors/IRead.tspackages/apps-engine/src/definition/mediaCalls/index.tspackages/apps/tests/server/accessors/Reader.test.tspackages/apps/tests/test-data/utilities.tspackages/apps/src/server/bridges/AppBridges.tspackages/apps/tests/test-data/bridges/mediaCallBridge.tspackages/apps/tests/test-data/bridges/appBridges.tspackages/apps-engine/src/definition/mediaCalls/IMediaCall.tspackages/apps/src/server/bridges/MediaCallBridge.tspackages/apps/src/server/managers/AppAccessorManager.tsapps/meteor/tests/data/apps/helper.tspackages/apps/src/server/accessors/Reader.tspackages/apps/deno-runtime/lib/accessors/mod.tspackages/apps/src/AppsEngine.tsapps/meteor/tests/end-to-end/apps/app-media-call-reader.ts
📚 Learning: 2026-05-06T12:21:44.083Z
Learnt from: juliajforesti
Repo: RocketChat/Rocket.Chat PR: 40256
File: apps/meteor/client/components/CreateDiscussion/CreateDiscussion.tsx:121-149
Timestamp: 2026-05-06T12:21:44.083Z
Learning: Field wrappers in rocket.chat/fuselage-forms (Field, FieldLabel, FieldRow, FieldError, FieldHint) auto-create htmlFor/id associations, aria-describedby, and role="alert" for errors. Do not manually set htmlFor, id, aria-describedby, or role attributes when using these wrappers. This automatic wiring does not apply to plain rocket.chat/fuselage components, which require explicit ID wiring per the accessibility docs. In code reviews, prefer using fuselage-forms wrappers for form fields and verify there is no unnecessary manual ID/aria wiring in files that use these wrappers. If a component uses plain fuselage components, ensure proper id wiring as per docs.
Applied to files:
packages/apps/src/server/accessors/index.tspackages/apps-engine/src/definition/accessors/index.tspackages/apps/tests/server/accessors/MediaCallRead.test.tspackages/apps-engine/src/definition/accessors/IMediaCallRead.tsapps/meteor/app/apps/server/bridges/mediaCalls.tsapps/meteor/tests/data/apps/app-packages/index.tspackages/apps/src/server/accessors/MediaCallRead.tspackages/apps/src/server/bridges/index.tspackages/apps/src/server/permissions/AppPermissions.tspackages/apps-engine/src/definition/accessors/IRead.tspackages/apps-engine/src/definition/mediaCalls/index.tspackages/apps/tests/server/accessors/Reader.test.tspackages/apps/tests/test-data/utilities.tspackages/apps/src/server/bridges/AppBridges.tspackages/apps/tests/test-data/bridges/mediaCallBridge.tspackages/apps/tests/test-data/bridges/appBridges.tspackages/apps-engine/src/definition/mediaCalls/IMediaCall.tspackages/apps/src/server/bridges/MediaCallBridge.tspackages/apps/src/server/managers/AppAccessorManager.tsapps/meteor/tests/data/apps/helper.tspackages/apps/src/server/accessors/Reader.tspackages/apps/deno-runtime/lib/accessors/mod.tspackages/apps/src/AppsEngine.tsapps/meteor/tests/end-to-end/apps/app-media-call-reader.ts
📚 Learning: 2026-05-11T21:46:23.471Z
Learnt from: d-gubert
Repo: RocketChat/Rocket.Chat PR: 40463
File: packages/apps/src/lib/SecureFields.ts:17-19
Timestamp: 2026-05-11T21:46:23.471Z
Learning: In Rocket.Chat’s `packages/apps/tsconfig.json`, TypeScript `"strict"` is set to `false`, which disables strict type-checking (including `noImplicitAny`) for `packages/apps`. When reviewing, do not flag TS7053 (and similar strict-mode indexing/type errors) in files under `packages/apps/src/` that are a consequence of this relaxed strictness—e.g., patterns like indexing an `unknown`/`object` via optional chaining such as `object?.[kSecureFields]`.
Applied to files:
packages/apps/src/server/accessors/index.tspackages/apps/src/server/accessors/MediaCallRead.tspackages/apps/src/server/bridges/index.tspackages/apps/src/server/permissions/AppPermissions.tspackages/apps/src/server/bridges/AppBridges.tspackages/apps/src/server/bridges/MediaCallBridge.tspackages/apps/src/server/managers/AppAccessorManager.tspackages/apps/src/server/accessors/Reader.tspackages/apps/src/AppsEngine.ts
📚 Learning: 2026-05-06T20:47:53.078Z
Learnt from: d-gubert
Repo: RocketChat/Rocket.Chat PR: 40186
File: apps/meteor/app/apps/server/bridges/uiInteraction.ts:2-2
Timestamp: 2026-05-06T20:47:53.078Z
Learning: Deep imports must be used in this repository because Meteor’s bundler does not respect package.json exports subpath mappings. Import using deep paths (e.g., rocket.chat/apps/dist/server/bridges/UiInteractionBridge) instead of relying on exports. Do not suggest or apply changes to exports maps in Meteor-consuming packages (e.g., packages/apps/package.json) as a fix for deep imports. This guideline applies to all TypeScript files under apps/meteor/app/apps/server/bridges.
Applied to files:
apps/meteor/app/apps/server/bridges/mediaCalls.ts
📚 Learning: 2026-03-16T21:50:37.589Z
Learnt from: amitb0ra
Repo: RocketChat/Rocket.Chat PR: 39676
File: .changeset/migrate-users-register-openapi.md:3-3
Timestamp: 2026-03-16T21:50:37.589Z
Learning: For changes related to OpenAPI migrations in Rocket.Chat/OpenAPI, when removing endpoint types and validators from rocket.chat/rest-typings (e.g., UserRegisterParamsPOST, /v1/users.register) document this as a minor changeset (not breaking) per RocketChat/Rocket.Chat-Open-API#150 Rule 7. Note that the endpoint type is re-exposed via a module augmentation .d.ts in the consuming package (e.g., packages/web-ui-registration/src/users-register.d.ts). In reviews, ensure the changeset clearly states: this is a non-breaking change, the major version should not be bumped, and the changeset reflects a minor version bump. Do not treat this as a breaking change during OpenAPI migrations.
Applied to files:
.changeset/tired-teeth-cheat.md
📚 Learning: 2026-02-04T12:08:56.950Z
Learnt from: d-gubert
Repo: RocketChat/Rocket.Chat PR: 38374
File: apps/meteor/tests/end-to-end/apps/app-logs-nested-requests.ts:26-37
Timestamp: 2026-02-04T12:08:56.950Z
Learning: In the E2E tests under apps/meteor/tests/end-to-end/apps, prefer using a fixed sleep (e.g., 1s) instead of implementing polling or retry logic when waiting for asynchronous operations to complete. Tests should fail deterministically if the expected result isn't available after the sleep.
Applied to files:
apps/meteor/tests/end-to-end/apps/app-media-call-reader.ts
📚 Learning: 2026-03-16T11:50:11.087Z
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 39657
File: apps/meteor/tests/end-to-end/apps/app-resolve-visitor.ts:18-21
Timestamp: 2026-03-16T11:50:11.087Z
Learning: In end-to-end tests under apps/meteor/tests/end-to-end/apps, there is an established pattern: call createAgent() and makeAgentAvailable() immediately after updateSetting('Livechat_enabled', true) with no intermediate delay. Do not flag this sequence as a race condition or require a sleep/delay. This pattern is used across 10+ tests in the codebase.
Applied to files:
apps/meteor/tests/end-to-end/apps/app-media-call-reader.ts
🪛 Biome (2.4.16)
apps/meteor/app/apps/server/bridges/bridges.js
[error] 17-17: Illegal use of an import declaration outside of a module
(parse)
🔇 Additional comments (27)
apps/meteor/app/apps/server/bridges/bridges.js (1)
17-17: LGTM!Also applies to: 65-65, 180-182
apps/meteor/app/apps/server/bridges/mediaCalls.ts (1)
1-16: LGTM!packages/apps/src/server/accessors/MediaCallRead.ts (1)
1-15: LGTM!packages/apps/src/server/accessors/Reader.ts (1)
6-6: LGTM!Also applies to: 38-38, 101-103
packages/apps/src/server/accessors/index.ts (1)
12-12: LGTM!Also applies to: 62-62
packages/apps/src/server/managers/AppAccessorManager.ts (1)
25-25: LGTM!Also applies to: 196-197, 216-216
packages/apps/deno-runtime/lib/accessors/mod.ts (1)
257-257: LGTM!packages/apps/src/AppsEngine.ts (1)
11-11: LGTM!packages/apps-engine/src/definition/mediaCalls/IMediaCall.ts (1)
1-41: LGTM!packages/apps-engine/src/definition/mediaCalls/index.ts (1)
1-1: LGTM!packages/apps-engine/src/definition/accessors/IRead.ts (1)
6-6: LGTM!Also applies to: 59-59
packages/apps-engine/src/definition/accessors/index.ts (1)
22-22: LGTM!packages/apps/src/server/bridges/AppBridges.ts (1)
15-15: LGTM!Also applies to: 56-57, 116-116
packages/apps/src/server/bridges/index.ts (1)
17-17: LGTM!Also applies to: 39-39
packages/apps/src/server/permissions/AppPermissions.ts (1)
132-134: LGTM!packages/apps/tests/test-data/utilities.ts (1)
508-542: LGTM!packages/apps/tests/test-data/bridges/appBridges.ts (1)
15-15: LGTM!Also applies to: 40-40, 115-115, 146-146, 261-263
packages/apps/tests/server/accessors/MediaCallRead.test.ts (1)
11-27: LGTM!packages/apps/tests/server/accessors/Reader.test.ts (1)
9-9: LGTM!Also applies to: 41-41, 45-66, 78-78
apps/meteor/tests/data/apps/app-packages/index.ts (1)
17-17: LGTM!apps/meteor/tests/data/apps/helper.ts (3)
7-35: Good error handling and async ZIP reading.The implementation correctly wraps the callback-based
readAsTextAsyncin a promise usingPromise.withResolvers(), and provides clear error context by wrapping the caught exception.
73-87: LGTM!
17-17:Promise.withResolvers()is supported by the project’s declared Node.js version.The repository pins Node.js to 22.22.2 via
engines.nodeinpackage.json(root andpackages/message-parser) and usesnode:22.22.2-*in Dockerfiles;Promise.withResolvers()is available in Node.js 22.0.0+.apps/meteor/tests/end-to-end/apps/app-media-call-reader.ts (3)
29-54: Comprehensive assertions for internal call shape.The test thoroughly validates the returned call object structure, including nested properties (createdBy.type, caller.type, callee.type) and array lengths (uids, features).
56-77: Comprehensive assertions for external call shape.Correctly validates external call differences: sipCallId presence, callee type 'sip', and single UID.
92-102: Correctly tests permission enforcement.Verifies that without
media-call.readpermission, the API returns{ call: null }instead of the call data, confirming the permission gate works as expected.apps/meteor/tests/data/apps/app-packages/README.md (1)
670-740: LGTM!
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
d-gubert
force-pushed
the
feat/sip-metadata
branch
from
56e3c52 to
a907d37
Compare
d-gubert
force-pushed
the
feat/apps-media-calls
branch
from
acc577e to
e5412dc
Compare
coderabbitai
Bot
removed
the
type: feature
d-gubert
force-pushed
the
feat/sip-metadata
branch
2 times, most recently
from
13f3355 to
243bdbc
Compare
fix(apps): align type definitions chore(apps): better type alignment
chore: better wording for changeset
d-gubert
force-pushed
the
feat/apps-media-calls
branch
from
e5412dc to
93430f7
Compare
coderabbitai
Bot
added
the
type: feature
2 participants
Proposed changes (including videos or screenshots)
Issue(s)
DMV-13
Steps to test or reproduce
Further comments
Summary by CodeRabbit
Release Notes
New Features
MediaCallReadaccessormedia-call.readpermission can retrieve call details by ID, including metadata, participants, and call stateTests